feat: Implemented GlobalThreadPool for async functions by mkopcins · Pull Request #603 · software-mansion/react-native-executorch

mkopcins · 2025-09-15T10:56:45Z

Description

Added singleton class GlobalThreadPool for single threadpool management so that we don't have to spawn new threads for each async function and instead we can delegate functions to the threadpool.

Also added pthreadpool and cpuinfo binaries for iOS to allow for XNNPack threadpool configuration just like on Android

Introduces a breaking change?

Yes
No

Type of change

Bug fix (change which fixes an issue)
New feature (change which adds functionality)
Documentation update (improves or adds clarity to existing documentation)
Other (chores, tests, code style improvements etc.)

Tested on

iOS
Android

Testing instructions

Screenshots

Related issues

Checklist

I have performed a self-review of my code
I have commented my code, particularly in hard-to-understand areas
I have updated the documentation accordingly
My changes generate no new warnings

Additional notes

msluszniak · 2025-09-15T12:42:13Z

+      WorkItem item;
+      item.task = std::move(task);
+      item.priority = priority;
+      item.enqueueTime = std::chrono::steady_clock::now();


This is completely up to you, but I would probably define constructor for this struct, it's not classic POD because you have a non-copyable std::unique_ptr here. What are the benefits? More concise initialization, priority and enqueueTime might be marked private.

jakmro · 2025-09-15T14:48:11Z

+namespace rnexecutorch {
+namespace threads {
+
+class GlobalThreadPool {


Consider removing this class and leaving all functions inside the threads::global_thread_pool namespace

HighPerformanceThreadPool should be probably implemented similarly to this one:

class Singleton { public: Singleton(const Singleton&) = delete; Singleton& operator=(const Singleton&) = delete; Singleton(Singleton&&) = delete; Singleton& operator=(Singleton&&) = delete; static Singleton& getInstance() { static std::once_flag initFlag; if (!instance) { throw std::logic_error("Instance not initialized."); } return *instance; } static void initialize(int arg) { static std::once_flag initFlag; std::call_once(initFlag, [arg]() { instance = std::make_unique<Singleton>(arg); }); } private: int value; Singleton(int val) : value(val) {} static std::unique_ptr<Singleton> instance; }; std::unique_ptr<Singleton> Singleton::instance = nullptr;

I think you meant GlobalThreadPool, right? HighPerformanceThreadPool was never supposed to be a singleton

Currently, HighPerformanceThreadPool is used only in the context of GlobalThreadPool. I guess Jakub meant that GlobalThreadPool is only a wrapper on a class that might be a singleton itself. If in the future there might be any other way to use HighPerformanceThreadPool the current design will make more sense to me then.

That was my idea when creating this, HighPerformanceThreadPool is not meant to be a singleton, but GlobalThreadPool is. Also semantically it makes more sense to me to keep GlobalThreadPool as a class instead of a namespace even though it doesn't have any nonstatic members.

msluszniak · 2025-09-15T16:33:44Z

Add conditional closing of this issue: #565 in the PR

msluszniak

Very small NITs

msluszniak

🚀

msluszniak · 2025-09-18T12:05:43Z

+
+  void setCPUAffinity() {
+    // AFAIK it is not possible on iOS
+#ifdef __ANDROID__


You should wrap the whole function implementation with #ifdef __ANDROID__ to not introduce overhead of translating empty function body on iOS. Check other applicable places

msluszniak · 2025-09-18T12:09:33Z

+  void setThreadPriority() {
+    // pthread_setschedparam doesn't work on android because permissions reasons
+    // and in general does not provide visible improvements on iOS
+
+    // Set nice value as fallback or additional priority boost
+    constexpr int nice_value = 0;
+    if (setpriority(PRIO_PROCESS, 0, nice_value) != 0) {
+      log(LOG_LEVEL::Debug, "Failed to set nice value");
+    } else {
+      log(LOG_LEVEL::Debug, "Set nice value", nice_value);
+    }
+  }


Some functions like here or getCurrentThreadId might be marked static or moved outside of the class

chmjkb

besides the single comment lgtm

chmjkb · 2025-09-22T06:24:04Z

+      '"$(PODS_TARGET_SRCROOT)/third-party/executorch/backends/xnnpack/third-party/pthreadpool/include" '+
+      '"$(PODS_TARGET_SRCROOT)/third-party/executorch/backends/xnnpack/third-party/cpuinfo/include" ',


Shouldn't this be third-party/include/executorch/...? If this is the case then it is already included in the line above

… iOS

## Description Added singleton class GlobalThreadPool for single threadpool management so that we don't have to spawn new threads for each async function and instead we can delegate functions to the threadpool. Also added pthreadpool and cpuinfo binaries for iOS to allow for XNNPack threadpool configuration just like on Android ### Introduces a breaking change? - [ ] Yes - [x] No ### Type of change - [ ] Bug fix (change which fixes an issue) - [x] New feature (change which adds functionality) - [ ] Documentation update (improves or adds clarity to existing documentation) - [ ] Other (chores, tests, code style improvements etc.) ### Tested on - [x] iOS - [x] Android ### Testing instructions  ### Screenshots  ### Related issues  ### Checklist - [ ] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have updated the documentation accordingly - [ ] My changes generate no new warnings ### Additional notes  --------- Co-authored-by: Mateusz Kopciński <mateusz.kopcinski@swmansnion.com>

Added singleton class GlobalThreadPool for single threadpool management so that we don't have to spawn new threads for each async function and instead we can delegate functions to the threadpool. Also added pthreadpool and cpuinfo binaries for iOS to allow for XNNPack threadpool configuration just like on Android - [ ] Yes - [x] No - [ ] Bug fix (change which fixes an issue) - [x] New feature (change which adds functionality) - [ ] Documentation update (improves or adds clarity to existing documentation) - [ ] Other (chores, tests, code style improvements etc.) - [x] iOS - [x] Android    - [ ] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have updated the documentation accordingly - [ ] My changes generate no new warnings  --------- Co-authored-by: Mateusz Kopciński <mateusz.kopcinski@swmansnion.com>

…ion#603) ## Description Added singleton class GlobalThreadPool for single threadpool management so that we don't have to spawn new threads for each async function and instead we can delegate functions to the threadpool. Also added pthreadpool and cpuinfo binaries for iOS to allow for XNNPack threadpool configuration just like on Android ### Introduces a breaking change? - [ ] Yes - [x] No ### Type of change - [ ] Bug fix (change which fixes an issue) - [x] New feature (change which adds functionality) - [ ] Documentation update (improves or adds clarity to existing documentation) - [ ] Other (chores, tests, code style improvements etc.) ### Tested on - [x] iOS - [x] Android ### Testing instructions  ### Screenshots  ### Related issues  ### Checklist - [ ] I have performed a self-review of my code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have updated the documentation accordingly - [ ] My changes generate no new warnings ### Additional notes  --------- Co-authored-by: Mateusz Kopciński <mateusz.kopcinski@swmansnion.com>

mkopcins force-pushed the @mkopcins/global-thread-pool branch 4 times, most recently from ca24036 to 18ea813 Compare September 15, 2025 11:27

mkopcins marked this pull request as ready for review September 15, 2025 11:28

mkopcins requested review from chmjkb and jakmro September 15, 2025 11:28

msluszniak reviewed Sep 15, 2025

View reviewed changes

Comment thread packages/react-native-executorch/common/rnexecutorch/threads/HighPerformanceThreadPool.h Outdated

msluszniak reviewed Sep 15, 2025

View reviewed changes

Comment thread packages/react-native-executorch/common/rnexecutorch/threads/HighPerformanceThreadPool.h Outdated

msluszniak reviewed Sep 15, 2025

View reviewed changes

chmjkb requested changes Sep 15, 2025

View reviewed changes

jakmro reviewed Sep 15, 2025

View reviewed changes

chmjkb requested changes Sep 16, 2025

View reviewed changes

msluszniak reviewed Sep 17, 2025

View reviewed changes

msluszniak approved these changes Sep 17, 2025

View reviewed changes

msluszniak reviewed Sep 18, 2025

View reviewed changes

chmjkb reviewed Sep 22, 2025

View reviewed changes

mkopcins force-pushed the @mkopcins/global-thread-pool branch from 159141f to 6912d47 Compare September 23, 2025 11:24

Mateusz Kopciński added 7 commits September 24, 2025 10:27

Initial work for global thread pool

b37a2cc

added threadpool extension, partially fixed perf problems

aa40e59

Initial changes for iOS compliance

c42df70

Implemented GlobalThreadPool and added setting xnnpack threadpool for…

1d89f5f

… iOS

adressed review comments

c39e2a8

small review changes

b01235d

removed unnecessary include directory

5d4e588

mkopcins force-pushed the @mkopcins/global-thread-pool branch from 6912d47 to 5d4e588 Compare September 24, 2025 08:28

chmjkb approved these changes Sep 24, 2025

View reviewed changes

mkopcins merged commit 363e335 into main Sep 24, 2025
2 checks passed

mkopcins deleted the @mkopcins/global-thread-pool branch September 24, 2025 08:36

		'"$(PODS_TARGET_SRCROOT)/third-party/executorch/backends/xnnpack/third-party/pthreadpool/include" '+
		'"$(PODS_TARGET_SRCROOT)/third-party/executorch/backends/xnnpack/third-party/cpuinfo/include" ',

Conversation

mkopcins commented Sep 15, 2025

Description

Introduces a breaking change?

Type of change

Tested on

Testing instructions

Screenshots

Related issues

Checklist

Additional notes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

msluszniak Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jakmro Sep 15, 2025

Choose a reason for hiding this comment

Uh oh!

msluszniak Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkopcins Sep 16, 2025

Choose a reason for hiding this comment

Uh oh!

msluszniak Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mkopcins Sep 17, 2025

Choose a reason for hiding this comment

Uh oh!

msluszniak commented Sep 15, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

msluszniak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

msluszniak left a comment

Choose a reason for hiding this comment

Uh oh!

msluszniak Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

msluszniak Sep 18, 2025

Choose a reason for hiding this comment

Uh oh!

chmjkb left a comment

Choose a reason for hiding this comment

Uh oh!

chmjkb Sep 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

msluszniak Sep 15, 2025 •

edited

Loading

msluszniak Sep 15, 2025 •

edited

Loading

msluszniak Sep 16, 2025 •

edited

Loading